I was expecting this to be a draft PR, or a PR against a feature branch. What is the intention of this @Ellerbach? This would have to go through APIReview first before it can get merged.

This would have to go through APIReview first before it can get merged

That's fine. I can mark the PR as Draft then. If you prefer, I can just push the branch directly to the repo so it will be its own feature branch.

@joperezr can we do small API review during triage meeting? This API is not part of the framework and we didn't go through API review for other smaller things

If the breaking change is acceptable for 3.0 and well documented, it's fine to wait for that version to be out. In the mean time, it does allow time to align on the design.

I don't think this is a small thing to be honest. This was one of the biggest design decisions made on the original design, and I do think that getting this reviewed by more people would help as most of us are already a bit biased.

Some notes (+ my after-thoughts) from the offline conversation between me and @Ellerbach today:

• abstract class/interface has benefits of being able to remove all validation code and do some pre-calculations and really save time
• interface can be implemented with struct which has extra benefit of being able to efficiently store list of pins if used directly
• class implementing interface/abstract class when used directly rather than base type it will not have virtual cost (although such usage might be much harder)
• two interesting opposite use cases: LED Matrix (stretch of the perf), LCD character screen (many pins which might need to come from different controllers)
• non-abstract/no-interface can also remove extra validation if i.e. using non-validating driver overload and since we pre-validate that's safe
• pin management makes things much safer so non-validating pin might not always be a good idea
• if we cannot make specialized abstract class work fast enough with i.e. LED Matrix then why bother making it abstract if it doesn't solve largest use case for that?
• LED matrix is currently using registers directly, perhaps improving that interface to work with other controllers than just RPi3 might be a better idea for perf scenarios than Pin class - possibly register should always return a pointer and be part of GpioController/GpioDriver - this way pref-interested parties can use that, for drivers which don't support that it would either throw or return null sentinel
• in general we should do some experimentation here and we're in agreement that low-level concepts are really hard to get right in high level APIs and solving both perf and safety at the same time might not be possible so we should find a good sweet spot between or shift to one side

Few elements to take into consideration:

• The changes are mainly about removing the sealed so it will allow to create other GpioPin based on a driver for example if needed
• From the conversation mentioned up, having an abstract class does not need to be necessary as we will use the controller in almost all cases

Why not PinValue Toggle(int pinNumber)?

Hardware does not provide. I know that for all current implementation we keep track, so could be provided but in real hardware, it's not.

We were discussing in the triage to shape the api as PinValue? Toggle(int pinNumber).
My point was the following:

• In the software implementation returning the value is free because we track the values
• In the hardware implementation providing the value, we return it
• In the hardware implementation not providing the value, we can return null (telling that is not available)

@joperezr Do you have an opinion on this?